Shared-layer lifecycle: accounting, eviction, recovery, materialization - #458
Shared-layer lifecycle: accounting, eviction, recovery, materialization#458chruffins wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 33c8ca3. Configure here.
| info, statErr := os.Stat(dirPath) | ||
| if statErr != nil || info.ModTime().After(cutoff) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Eviction can wipe in-flight layers
Medium Severity
Layer eviction decides freshness from the digest directory ModTime, and it runs concurrently with materialization. Unpack and mkfs.erofs write inside child temp dirs, so the parent mtime goes stale; DeleteImage can then RemoveAll that tree while a build is still using it, especially during the later unlocked ExportRootfs window before the manifest is written.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 33c8ca3. Configure here.
| if result.CacheHit { | ||
| cacheStatus = "hit" | ||
| } | ||
| m.recordImageBuildPhase(ctx, ref.Digest(), "layer_materialization", time.Since(materializeStart), "success", cacheStatus) |
There was a problem hiding this comment.
Layer bytes skipped after failed builds
Medium Severity
Materialized layer artifacts are written to the layer store before conversion, but refreshDiskUsageTotals only runs on successful finalize, delete, or startup. After a failed conversion the artifacts remain on disk while TotalImageBytes keeps the stale cached total, so capacity admission undercounts the real footprint.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 33c8ca3. Configure here.
41f4080 to
35de95e
Compare
35de95e to
f9a5fcc
Compare
d9caa0e to
b0a27b4
Compare
b0a27b4 to
37b8128
Compare
37b8128 to
13adf76
Compare
13adf76 to
c0e6c32
Compare
c0e6c32 to
aac8b57
Compare
aac8b57 to
7464627
Compare
4467fe9 to
588fff8
Compare
ac3de8e to
165e42f
Compare
165e42f to
3cac19c
Compare
8ca57c1 to
419c294
Compare
c238f03 to
5051c46
Compare
5051c46 to
53b68ce
Compare
4578e67 to
1dddcc5
Compare
1dddcc5 to
aba220f
Compare
-->
✱ stlc build✅ go code · compare
✅ python code · compare
✅ typescript code · compare
Diagnostics: ❗ 0 new / 1 total error, 💡 0 new / 5 total note
Build metadata
This comment is auto-generated by stlc and is kept up to date as you push. |
aba220f to
f0c7189
Compare
f0c7189 to
3cf0501
Compare
3cf0501 to
10f5e38
Compare
10f5e38 to
9cbf5fb
Compare
466f089 to
0fa6aa6
Compare
78e404c to
0e1b801
Compare
0e1b801 to
888f303
Compare
summary
Capstone stage of the image-storage project — finishes migration, eviction, accounting, recovery, and observability on the shared-layer path.
images/layers/<digest>/(best effort; the composed rootfs still comes from blobs, so an artifact failure only degrades sharing). Newlayer_materializationbuild phase with cache-hit attribution..unpack-*/.install-*temp directories are swept at startup (age-gated so live builds are untouched).TotalImageBytesnow includes the layer store's physical bytes (TotalLayerBytesexposes them separately), so capacity admission sees the real footprint.hypeman_images_layer_artifacts_evicted_totalcounter plus slog eviction summaries and per-layer materialization warnings.validation
go test ./lib/images ./lib/paths ./lib/builds ./lib/scopesgreen;cmd/api/apigreen except Docker Hub pulls (anonymous rate limit — confirmed via direct probe returning TOOMANYREQUESTS) and VM lifecycle tests (need bridge privileges; verified failing identically on unmodified main).Note
Medium Risk
Changes image deletion, startup cleanup, and disk admission accounting; incorrect eviction could remove data still needed by images, though manifest references and a grace period mitigate that.
Overview
Completes the shared-layer storage path: pulled images materialize per-layer artifacts under
images/layers/(best effort, with alayer_materializationbuild phase), and reference-aware eviction removes layer dirs only when no manifest model still references them.Deletion and startup run eviction plus a sweep of stale
.unpack-*/.install-*temps, gated by a 10-minute grace period so in-flight builds are not raced.TotalImageBytesnow counts ready rootfs metadata plus physical layer-store bytes (TotalLayerBytesexposes layers alone); disk-usage caching tracks the newlayerBytescomponent.Adds
hypeman_images_layer_artifacts_evicted_totaland lifecycle tests (shared base layer materialized once, survives partial deletes, full eviction when the last reference goes).Reviewed by Cursor Bugbot for commit 33c8ca3. Configure here.